Skip to content

fix(openai): Guard against choices=None#6216

Merged
alexander-alderman-webb merged 13 commits intogetsentry:masterfrom
cla7aye15I4nd:fix/openai-choices-none
May 8, 2026
Merged

fix(openai): Guard against choices=None#6216
alexander-alderman-webb merged 13 commits intogetsentry:masterfrom
cla7aye15I4nd:fix/openai-choices-none

Conversation

@cla7aye15I4nd
Copy link
Copy Markdown
Contributor

@cla7aye15I4nd cla7aye15I4nd commented May 6, 2026

Summary

Some providers (e.g. OpenRouter) can return choices=None on upstream error responses. The existing hasattr(response, "choices") check passes because the attribute exists — it just doesn't guard against None.

This causes two distinct bugs:

1. Non-streaming: crash in caller

TypeError: 'NoneType' object is not iterable
  File "sentry_sdk/integrations/openai.py", in _calculate_token_usage
    for choice in response.choices:

This propagates out of Sentry's patched create() wrapper before the calling code ever receives the response, making it impossible to handle on the caller side without catching a bare TypeError.

2. Streaming: silent data loss in span

Inside _wrap_synchronous_completions_chunk_iterator and _wrap_asynchronous_completions_chunk_iterator, the iteration is wrapped in capture_internal_exceptions(). When choices=None, the TypeError is silently suppressed — but this causes a premature exit from the with block, so streaming_message_total_token_usage = x.usage is never reached. Token usage and response text are then missing from the Sentry span.

Fix

Add an explicit is not None guard alongside the existing hasattr check at all four affected locations (non-streaming, sync streaming, async streaming).

Reproduction

Use the OpenAI SDK client pointed at OpenRouter (base_url="https://openrouter.ai/api/v1"). When an upstream provider fails, OpenRouter returns a response with choices=null, which triggers both bugs.

Some providers (e.g. OpenRouter) can return choices=None on upstream
error responses. hasattr(response, 'choices') returns True even when
the attribute is None, causing a TypeError when iterating.

Add an explicit None check before iterating response.choices.
@cla7aye15I4nd cla7aye15I4nd requested a review from a team as a code owner May 6, 2026 22:55
Comment thread sentry_sdk/integrations/openai.py
cla7aye15I4nd and others added 7 commits May 6, 2026 16:01
When a streaming chunk has choices=None, the existing hasattr check passes
but the subsequent iteration raises TypeError. Inside capture_internal_exceptions()
this is silently suppressed, causing the usage capture on the next line to be
skipped — leaving token usage and response text missing from the Sentry span.

Add explicit None checks in both the sync and async streaming iterators.
…getsentry#6202)

The HTTP client span in the stdlib integration was finishing in
`getresponse()`, which only waits for response headers, not the actual
response. For chunked or large responses, the actual data transfer
happens during `read()`, leaving that time uninstrumented.

Defer span completion to `HTTPResponse.read()` for responses with a body
(chunked or Content-Length > 0), with `HTTPResponse.close()` as a safety
net for responses that are never read.

⚠️ Note that this means we might report some requests to be longer than
they actually were, since in some cases we only close them once the GC
gets to them (and `close()` is called). In a sense we're essentially
flipping the current situation (where we report requests to be much
shorter than they are, since they don't include the response part) --
but the current situation was incorrect for all spans, while this
`close()` fallback should hopefully only kick in for edge cases.

Responses with no body (Content-Length: 0, HEAD, 204, 304) still finish
the span immediately in `getresponse()`.

* Closes getsentry#2277
* Closes
https://linear.app/getsentry/issue/PY-159/sentry-does-not-fully-instrument-requests-library-requests

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Alex Alderman Webb <alexander.webb@sentry.io>
…ms` (getsentry#6233)

We had multiple `envelopes_to_x` helpers around in tests for unwrapping
metrics, logs, and spans. Replace them all with `capture_items`, which
has the added benefit of being more true to the actual payload leaving
the SDK, unlike `envelopes_to_x`, which used to modify some fields
(setting them to `None` if they were not present, for instance).

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
### Description
Reintroducing the argv integration port from
getsentry#6147. Work on full
support for array attributes in the product is still ongoing, but at
least the trace will now load (the attribute won't be displayed), so
we're not breaking anything with this change.

#### Issues
- Closes getsentry#5998
- Closes
https://linear.app/getsentry/issue/PY-2300/migrate-argv-to-span-first

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)
### Description
See getsentry#6217

#### Issues
* Resolves
https://linear.app/getsentry/issue/PY-2407/force-a-new-segment-where-we-were-creating-transactions
* Resolves getsentry#6217

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)
@alexander-alderman-webb
Copy link
Copy Markdown
Contributor

alexander-alderman-webb commented May 7, 2026

Hi @cla7aye15I4nd,

Thanks a lot for the PR! We seem to have pulled in a new mypy major so merging is blocked by our required checks (very unfortunate timing).
I'll give you a few days to update your PR (merge in master or rebase), and otherwise I'll take over.
The type failures are all fixed in bd567bd.

sentrivana and others added 4 commits May 7, 2026 14:33
### Description

Set
[`flag.evaluation.{key}`](https://getsentry.github.io/sentry-conventions/attributes/flag/#flag-evaluation-key)
as span attribute in span streaming mode.

Add a span streaming test variant to affected integrations:
- Unleash
- Statsig
- Launchdarkly
- OpenFeature

#### Issues
- Closes
https://linear.app/getsentry/issue/PY-2141/support-feature-flags-in-span-first
- Closes getsentry#5676
- Closes
https://linear.app/getsentry/issue/PY-2372/migrate-unleash-to-span-first
- Closes getsentry#6070
- Closes
https://linear.app/getsentry/issue/PY-2364/migrate-statsig-to-span-first
- Closes getsentry#6062
- Closes
https://linear.app/getsentry/issue/PY-2335/migrate-launchdarkly-to-span-first
- Closes getsentry#6033
- Closes
https://linear.app/getsentry/issue/PY-2344/migrate-openfeature-to-span-first
- Closes getsentry#6042

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)
…etsentry#6236)

Changing `sentry_sdk.trace` to `sentry_sdk.traces.trace` introduced a
whole avalanche of [typing
issues](https://github.com/getsentry/seer/actions/runs/25495762136/job/74814929830?pr=6291).

Adding the overloads we have for the original `trace`.
The field capturing the end timestamp of a span used to be called
`timestamp` in legacy mode (on the `Span` class), with `start_timestamp`
as its counterpart.

In span streaming, we originally followed the same convention on the new
`StreamedSpan` class, but there's actually no reason to and it'd be
better to use `end_timestamp` instead:
- the field on the serialized span v2 is actually called `end_timestamp`
- `end_timestamp` is a clearer name in general, and complements
`start_timestamp` nicely
@cla7aye15I4nd cla7aye15I4nd force-pushed the fix/openai-choices-none branch 2 times, most recently from 1387d00 to 8a5b079 Compare May 7, 2026 19:50
@cla7aye15I4nd
Copy link
Copy Markdown
Contributor Author

Thanks you ! I have fixed.

@alexander-alderman-webb alexander-alderman-webb changed the title fix(openai): guard against choices=None in OpenAI integration fix(openai): Guard against choices=None May 8, 2026
@alexander-alderman-webb alexander-alderman-webb merged commit 640c48d into getsentry:master May 8, 2026
150 of 151 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants